Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removal of all FS code and member renaming #150

Merged
merged 4 commits into from
Jan 5, 2024
Merged

Removal of all FS code and member renaming #150

merged 4 commits into from
Jan 5, 2024

Conversation

Meziu
Copy link
Member

@Meziu Meziu commented Jan 2, 2024

Inspired by this comment.

The fs service module is the only piece of this repo that hasn't received any form of update since the evolution from the old implementation.

Much of the reason why I decided to change the approach on the Rust3DS toolchain was to align as much as possible with the official Rust APIs and guidelines, so that we could embrace as much of the public ecosystem as possible. However, the fs module has for long been left behind as it wasn't really a priority (thanks to the already existing compatibility with C files for the SD card and ROMFS) and as such received no real improvements or testing.

What I removed in this PR was the original re-write of the std::fs API that was made to closely resemble the standard Rust experience. At this point, all this code (which was still unable to communicate with any system Archives other than SDMC) results completely pointless to keep around (and even quite confusing).

If we ever need to implement the general libctru file-system functionality into ctru-rs, we should do it by either writing a completely new API that fits the needs nicely, or work to make the already existing std::fs API interoperable with more Archives, just how it already works with SDMC and ROMFS (an idea that was already thought of in the old days).

@Meziu Meziu requested a review from a team as a code owner January 2, 2024 12:19
ctru-rs/src/services/fs.rs Show resolved Hide resolved
ctru-rs/src/services/fs.rs Show resolved Hide resolved
@ian-h-chamberlain
Copy link
Member

Btw, it looks like there might be some renewed interest in updated std::fs::path APIs for prefixes too (rust-lang/rust#52331), which would be nice if we could develop some kinds of PathExt, FileExt in the upstream stdlibs.

@Meziu Meziu merged commit b1021c5 into master Jan 5, 2024
4 checks passed
@Meziu Meziu deleted the remove/fs branch January 5, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants